fix(refund-vault): add domain separator and monotonic nonce (#136) - #252
Merged
Merged
Conversation
) Add cryptographic hardening to RefundVault: a domain separator (get_domain_separator) stored as SHA-256 of the contract address at initialisation, and a monotonic operation nonce (get_nonce) incremented on every successful state-changing call. Events now carry the nonce so off-chain indexers can detect replays or reorderings. Separate vault instances produce distinct domain separators, preventing cross-contract replay of signed authorisations. Tests verify nonce monotonicity, cross-instance separator uniqueness, and nonce absence on failed calls. Fixes accensa#136 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
MergeKeeper review Scope: in scope for linked issue The pull request correctly implements the domain separator and monotonic nonce hardening in RefundVault alongside thorough test updates. Reviewed commit: |
|
MergeKeeper merge status Status: blocked Reason: One or more required CI checks failed. Failing checks:
Next steps:
|
- Replace Address::to_buffer() with Into<Bytes> conversion (API not available in soroban-sdk 27) - Cast u64 nonce literals to i128 in soroban map! macro - Add extern crate std for no_std test compatibility - Add missing Events trait imports for self-transfer tests - Prefix unused variables with underscore Fixes accensa#136 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
- Use soroban_sdk::String::from() for Address to Bytes conversion - Replace .len() with .events.len() on ContractEvents - Remove unused storage::Persistent import Fixes accensa#136 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
The assert_eq! and closing brace of test_process_batch_exceeds_max_size_fails were dropped in a bad merge, causing cargo fmt to fail with an unclosed delimiter error. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
# Conflicts: # CHANGELOG.md # contracts/refund-vault/src/lib.rs # contracts/refund-vault/src/test.rs
…ngle claim_single (from upstream/main) published RefundEvent without the nonce field that fix/issue-136 added to the struct, causing a compile error. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds cryptographic hardening to
RefundVaultin response to Issue #136 (Audit and Fix Cryptographic Malleability in RefundVault Signatures).Audit finding
RefundVault does not use custom signature verification — it relies entirely on Soroban's built-in
require_auth()mechanism (Ed25519, inherently canonical). There are no ECDSA paths to malleate.Hardening added (defense-in-depth)
Domain Separator (
get_domain_separator): A SHA-256 hash of the contract address, stored atinitializeand never changed. Off-chain systems should bind signed authorizations to this value so a replay against a different vault deployment is rejected. Separate instances produce distinct separators.Monotonic Nonce (
get_nonce): Au64counter incremented on every successful state-changing call (deposit,refund,withdraw,deploy_to_yield,withdraw_from_yield,harvest_yield). Events carry the nonce so off-chain indexers can detect replays or reorderings.Acceptance criteria
Tests added
test_domain_separator_is_set_on_initializetest_domain_separator_differs_per_instancetest_nonce_starts_at_zerotest_nonce_increments_on_deposittest_nonce_increments_on_refundtest_nonce_increments_on_withdrawtest_nonce_does_not_increment_on_failed_operationtest_nonce_is_strictly_monotonicEvent assertion tests updated to verify the nonce field.
Files changed
contracts/refund-vault/src/lib.rs— Nonce, DomainSeparator, event nonce fields, getterscontracts/refund-vault/src/test.rs— New domain-separator/nonce tests, updated event assertionscontracts/refund-vault/src/yield_tests.rs— Updated event assertions for nonce fieldCHANGELOG.md— Entry for issue Audit and Fix Cryptographic Malleability in RefundVault Signatures #136Fixes #136